Python Architect

• [recommended] Conflicted-lockfile outcome is split across call sites instead of one owner. at src/apm_cli/commands/install.py:1837
  Detection is centralized in LockFile.read; durable outcome (unlink vs fail closed vs warn) is recomputed at install, lock, frozen, dry-run, and MCP.
  Suggested: One lockfile conflict policy invoked from those call sites.
• [recommended] discard_conflicted_lockfile unlinks apm.lock.yaml outside InstallTransaction. at src/apm_cli/deps/lockfile.py:1283
  unlink before resolve; later install failure cannot restore the conflicted file via InstallTransaction.
• [nit] MCP install catches LockfileFormatError after writing apm.yml. at src/apm_cli/install/mcp/command.py:308
  Pre-existing order; conflicted lock can leave manifest write without regenerated lock.

CLI Logging Expert

No findings.

DevX UX Expert

• [recommended] Make regenerate vs fail-closed explicit for interactive vs CI
  The PR implements both regenerate-on-full-install and fail-closed for frozen/partial installs. Users and CI need a clear rule about which behavior is the default. CEO: accepted scope already covers this; do not add CI autodetection.
• [recommended] Clarify dry-run output and ensure machine-detectable exit codes
  Dry-run currently reports human-readable outcomes like "would make no changes" vs warnings. Author documented this limitation. CEO: keep warn + exit 0.
• [nit] Address non-transactional lockfile/regeneration risks in UX and docs
  Regenerating without atomic replace risks partial state if interrupted.

Supply Chain Security Expert

No findings.

Doc Writer

• [recommended] Do not over-claim that every install recovers a conflicted lockfile at docs/src/content/docs/reference/cli/install.md
  Full install/lock recover; frozen/partial fail closed; dry-run does not rewrite.
• [recommended] Revert unrelated hermes/target wording churn in commands.md at packages/apm-guide/.apm/skills/apm-usage/commands.md:15
  A lockfile-conflict patch should not rewrite the hermes/target table; duplicate install rows already disagree.
• [nit] State conflict semantics once; point other pages at lockfile-spec at docs/src/content/docs/reference/lockfile-spec.md
  Canonical definition belongs in lockfile-spec; recovery steps in install-failures.md.

Test Coverage Expert

• [blocking] No test that a failed install after discard/unlink leaves durable state safe
  Author-stated limitation: discard outside transaction.
  Proof (missing at): tests/integration/test_install_failed_after_discard_unlink.py
• [recommended] No explicit --force override test
  Product intent: --force is unchanged and is NOT a lock regenerate switch. CEO dropped this follow-up.
  Proof (missing at): tests/integration/test_install_force_override.py
• [recommended] No ApmLifecycle snapshot for unlink-then-fail
  Overlaps the unlink-then-fail coverage gap.
  Proof (missing at): tests/integration/test_lifecycle_unlink_then_fail_snapshot.py

Performance Expert

No findings.

Python Architect

• [recommended] Frozen handler in install/service.py hardcodes apm.lock.yaml and reconstructs the conflict diagnostic independently of LockfileConflictError, creating a second message authority. at src/apm_cli/install/service.py
  Single-owner rule (architecture.instructions.md): every durable decision has exactly one canonical owner; every call site routes through it. LockfileConflictError is the canonical owner of conflict diagnostic wording, path, and git-command format. The frozen handler at service.py:268-307 catches the typed error but discards its message, hardcodes the filename as 'apm.lock.yaml' instead of reading exc.path, and reconstructs the full prose ('--frozen cannot read apm.lock.yaml: it contains...') plus git commands independently. If the canonical error's wording, path.name derivation, or git command format changes, this handler silently diverges. Two authorities for one message is the pattern the single-owner rule exists to prevent. The frozen-specific prefix ('--frozen cannot read') and suffix ('then commit the result') are legitimate additions; they should compose around the canonical message, not replace it. This is not blocking because the hardcoded path is currently always correct for frozen installs (project_dir/apm.lock.yaml is the canonical lockfile location) and no correctness regression exists today -- the risk is silent divergence under future maintenance.

Design patterns -- Used in this PR: Base class + subclass (LockfileConflictError extending LockfileFormatError) for exception hierarchy; existing catch chains naturally specialize via ordered except clauses. Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope; the fix is to compose the frozen prefix/suffix around str(exc) or exc.path, not to add a new pattern.
Suggested: Compose the frozen-specific framing around the canonical error rather than reconstructing it: use exc.path.name for the filename and reference str(exc) or the error's core message for the conflict diagnostic, adding only the frozen-specific prefix and 'then commit the result' suffix.

DevX UX Expert

• [recommended] The Git-only recipe does not cover every file the marker detector diagnoses. at src/apm_cli/deps/lockfile.py:84
  Detection examines bytes, not Git index state. A copied/untracked conflict or a file whose committed contents already contain markers may have no usable merge side; checkout can fail or restore the same unreadable contents. The diagnostic supplies no manual-resolve or restore-known-good fallback. The current code was read, not executed in this review.
  Suggested: Lead with resolving the named file or restoring a known-good copy. Keep any Git-side selection as an optional, context-qualified example, then retry only after the file is readable.

• [recommended] The printed basename can refer to a different lockfile than the diagnosed path. at src/apm_cli/deps/lockfile.py:89
  Lock export may find an ancestor manifest and outdated --global reads the user-scope lockfile without changing directory. The error names that actual path but prints git checkout apm.lock.yaml relative to the caller's directory. That recipe can fail or act on a different local file. An ancestor project may itself be a Git repository; the issue is path targeting, not an assumption that it is outside Git.
  Suggested: Keep manual resolve/restore guidance tied to the actual diagnosed path; if showing a Git example, explicitly qualify the path and working directory.

Test Coverage Expert

Review unavailable: the synchronous task completed without a JSON return. No coverage or executed-test claim is inferred.

Swagger / OpenAPI Editor -- shocked_meter 8/10, confidence high

Summary: Clean single-requirement editorial-patch amendment. The normative text is internally consistent, counts match across all three locations, conformance class is correct, and the defensive non-authorization clause for automatic recovery is well-crafted. One substantive gap: the MUST-level obligation references an undefined term ('version-control merge conflict markers') that should be pinned to an observable byte pattern for interface-contract interoperability. Two editorial nits on keyword formality and section-heading staleness. No blocking findings.

New recommended findings (1)

• [sw-rec-r1-1] req-lk-023 / Section 3 -- req-lk-023 binds a MUST to 'version-control merge conflict markers' but the term is undefined in Section 3 (Terminology) or inline. Two conformant implementations could disagree on which byte patterns constitute a marker (e.g. diff3-style angle-bracket lines vs. Perforce-style markers). The non-normative companion lockfile-spec.md pins specific Git patterns (seven angle-brackets / pipes at line start), but the normative spec does not. For an interface contract this is an interoperability gap: clause (a) says MUST identify 'the markers' but does not define what to identify.
  Recommended fix: Add either (i) a Terminology row in Section 3 defining 'merge conflict marker' as any line beginning with seven consecutive angle-bracket or pipe characters followed by a space or end-of-line (the diff3 convention shared by Git, Mercurial, and SVN), or (ii) a parenthetical in the req-lk-023 preamble: '...carrying unresolved version-control merge conflict markers (lines beginning with seven left-angle-brackets, seven right-angle-brackets, or seven pipe characters, each followed by a space or end-of-line)'. Either approach pins the MUST to an observable pattern without over-fitting to a single VCS.

New nit findings (2)

• [sw-nit-r1-1] req-lk-023 clause (b) -- The tail of clause (b) reads 'naming one as a follow-up step, sequenced after the resolving action, is permitted.' The word 'permitted' functions as a normative grant but is not a BCP 14 keyword. Section 2 says lowercase variants carry no normative weight, so the sentence is technically non-normative -- yet it clearly intends to grant permission. Strict RFC 2119 / 8174 discipline would use MAY.
  Recommended fix: Rewrite the sentence: '...naming one as a follow-up step, sequenced after the resolving action, MAY be included in the diagnostic.'
• [sw-nit-r1-2] Section 5.4 heading -- The heading reads 'Lockfile versions (1, 2) and bumping rules' but now also houses req-lk-023 (conflict-marker recognition), which is not a versioning or bumping concern. The heading text is stale relative to the broadened section scope.
  Recommended fix: Broaden the heading to reflect all load-time gatekeeping, e.g. 'Lockfile versions, bumping rules, and load-time validation', or split req-lk-023 into its own subsection (5.4.1).

Preserved strengths confirmed

• Mechanical checks independently found 124 unique normative anchors and matching Section 1.3, Appendix C and latest Appendix D totals.
• Consumer MUST enumeration and explicit exclusion of automatic recovery are preserved.

OCI Distribution Editor -- shocked_meter 8/10, confidence high

Summary: The diagnosis-only amendment preserves the file and avoids raw parser output. The marker pattern set needs precision for consistent diagnostics; no security bypass was demonstrated.

New recommended findings (1)

• [oci-rec-r1-1] req-lk-023 -- The MUST refers to unresolved version-control merge conflict markers without defining recognizable byte patterns. The implementation excludes a bare ======= separator and scans opener, closer and diff3 forms, but this choice is not explicit in the normative text. Implementations can therefore disagree about which inputs receive the named conflict diagnosis. No security bypass was demonstrated.
  Recommended fix: Add a non-normative note immediately after the closing paragraph of req-lk-023, e.g.: 'Note -- For the purposes of this requirement, the canonical conflict marker set is the sequences <<<<<<< , >>>>>>> , and ||||||| appearing at the start of a line, each followed by a space or end-of-line. A bare ======= separator alone is not sufficient evidence of a conflict because a real version-control conflict always includes at least one labelled marker.' This keeps the MUST portable across VCS tooling while giving implementers a shared reference set for conformance testing.

New nit findings (1)

• [oci-nit-r1-1] sec.5.4 -- req-lk-023 is placed in Section 5.4 whose title is 'Lockfile versions (1, 2) and bumping rules'. Conflict-marker detection is a load-time validation concern, not a versioning or bumping concern. The Appendix C index correctly lists the section as 5.4, so a reader scanning by section title would miss the requirement.
  Recommended fix: No immediate action required; note for the next editorial pass that Section 5.4 could be retitled to 'Lockfile versions, load-time validation, and bumping rules' or that req-lk-023 could move to a dedicated subsection if the pre-parse validation surface grows.

Preserved strengths confirmed

• Clause (c) preserves the conflicted lockfile rather than introducing automatic discard or re-resolution.
• Clause (a) names path and cause without raw parser output.
• Synthesis correction: the new requirement does not impose a different exit policy, and Appendix D 0.1.42 does not state a Section 9.2 compatibility classification. Unsupported raw-return claims to those effects were not carried forward.

Package-Manager Editor -- shocked_meter 7/10, confidence high

Summary: Diagnostic-only scope and consumer classification are coherent. Marker grammar needs precision; the heading is stale. One Appendix D nit was discarded during synthesis because its premise was false.

New recommended findings (1)

• [pkg-rec-r1-1] req-lk-023 -- The requirement refers to unresolved version-control merge conflict markers without a minimum recognizable grammar. One implementation might diagnose a single labeled marker while another requires a complete opener/separator/closer group. The conformance oracle uses concrete Git-style input without defining the portable minimum in normative text. Existing best-effort readers and exit semantics are outside this ambiguity; this finding does not imply every command fails closed.
  Recommended fix: Add a parenthetical example list after the first mention in req-lk-023: 'unresolved version-control merge conflict markers (for example, a line beginning with seven less-than, greater-than, or pipe characters followed by a space or end-of-line)'. This aligns the normative text with the conformance oracle without excluding implementations that recognise additional VCS marker formats.

New nit findings (2)

• [pkg-nit-r1-1] sec.5.4 -- req-lk-023 is placed inside Section 5.4 whose heading is 'Lockfile versions (1, 2) and bumping rules.' The requirement governs conflict-marker diagnosis, not version semantics. While editorially adjacent to req-lk-004 (unrecognised lockfile_version), the heading misleads implementers scanning by section title for the marker-diagnosis contract.
  Recommended fix: Defer to a future editorial pass: rename Section 5.4 to 'Lockfile versions, bumping rules, and load-time diagnostics' or extract req-lk-023 into a new subsection 5.4.1.
• [pkg-nit-r1-2] Appendix D row 0.1.42 -- Not carried forward: this item misattributed companion Git-command wording to Appendix D. The actual row already says resolve the conflict or restore a known-good lockfile; no change to that row is warranted.

Preserved strengths confirmed

• Clause (c) MUST-NOT ('MUST NOT remove, rewrite, or re-resolve the lockfile in response to the markers') is a proper defensive reservation that requires a future amendment to lift, preventing silent recovery drift without an explicit spec change.
• Consumer-only conformance class assignment is correct; the requirement imposes no Producer, Registry, or Governance obligations, and the Appendix C, Section 5.7, and Section 11.3.2 enumerations are all consistent.
• The closing scope limiter ('this requirement neither defines nor authorises automatic recovery from a conflicted lockfile') explicitly defers recovery without foreclosing it, matching the reserved-slot pattern used for workspaces, conflict_resolution:nest, and other v0.2 deferrals.
• The diagnostic-advice clause (b) makes no false pin-preservation claim: 'resolve the conflict in that file or restore a known-good lockfile' honestly encompasses lossy single-side restoration (which discards the opposite side's recorded pins) alongside lossless manual merge, without promising that both parents' transitive resolutions survive.

TAG Architect -- shocked_meter 8/10, confidence high

Summary: req-lk-023 is well-structured, properly integrated across all conformance surfaces, and defensively scoped. Two recommended findings address cross-implementation clarity: the marker grammar is normatively referenced but never enumerated, and clause (a) embeds an untestable architectural constraint alongside testable behavioural requirements. Neither is blocking; both are cleanly deferrable to a v0.1.x followup.

New recommended findings (2)

• [tag-rec-r1-1] req-lk-023 clause (a) -- The normative text requires a consumer to recognise 'unresolved version-control merge conflict markers' but never enumerates or references a grammar for those markers. Git two-way conflicts use lines beginning with '<<<<<<< ' and '>>>>>>> '; diff3 mode adds '||||||| '; other VCS systems use different sequences. A third-party implementer reading only the spec cannot produce an interoperable marker scanner, and two conformant implementations may disagree on what constitutes a marker. The conformance test fixture exercises only one git-style pattern, reinforcing the gap between the normative prose and testable surface.
  Recommended fix: Add an informative parenthetical or non-normative note after 'version-control merge conflict markers' in clause (a), e.g.: '(in practice, lines beginning with the seven-character sequences "<<<<<<< ", ">>>>>>> ", and optionally "||||||| ", as emitted by git and compatible tooling)'. This keeps the normative MUST behavioural while giving implementers an enumerable minimum marker set to target.
• [tag-rec-r1-2] req-lk-023 clause (a) -- The phrase 'through the same authority that loads the lockfile' is an architectural constraint on implementation structure: it mandates that detection and loading share a code path. No conformance test can observe whether an implementation uses one code path or two -- only whether the observable output (correct detection, path in diagnostic, no raw parser spill) is correct. Normative statements should constrain observable behaviour rather than internal architecture, to avoid over-constraining implementations that use a pre-scan, a separate YAML front-end, or a streaming parser.
  Recommended fix: Replace 'through the same authority that loads the lockfile' with 'before or during the load that would otherwise attempt to parse the lockfile', or 'at lockfile-load time'. This preserves the temporal invariant (detection happens during the load phase, not later) without constraining implementation topology.

New nit findings (1)

• [tag-nit-r1-1] req-lk-023 / Section 5.4 -- req-lk-023 is placed in Section 5.4 ('Lockfile versions (1, 2) and bumping rules') but governs pre-parse conflict-marker detection, not version semantics. The Appendix C table cites section 5.4 accordingly. A reader scanning by section topic would not look under 'versions and bumping rules' for conflict-marker handling. The placement after req-lk-004 (pre-parse version rejection) is defensible as a 'load-time rejection' cluster, but the section title does not reflect this broader scope.
  Recommended fix: Consider a one-line parenthetical in the section heading or a brief editorial note acknowledging that req-lk-023 governs a load-time pre-parse condition co-located here with req-lk-004 for that reason. Alternatively, if a future revision introduces a 'Section 5.4.1 Load-time rejection' subsection, both requirements could migrate there.

Preserved strengths confirmed

• The new requirement, manifest entry, Appendix C row and conformance bindings agree on consumer classification.
• The closing scope paragraph excludes unrelated corruption and automatic recovery.
• Three conformance tests are bound to req-lk-023; this is static binding evidence, not an executed-test result.
• Synthesis correction: the new requirement does not impose a different exit policy, and Appendix D 0.1.42 does not state a Section 9.2 compatibility classification. Unsupported raw-return claims to those effects were not carried forward.

python-architect

• [recommended] Static guard contracts-tooling-lockfile-read covers filename resolution but not LockFile.read load authority or lock_export consumer at scripts/architecture_linter/checks/contracts_test_taxonomy.py:94
  The architecture boundary check check_lockfile_read_resolution (contracts_test_taxonomy.py:169) verifies that four enumerated consumers in _LOCKFILE_CONSUMERS (line 94) delegate filename resolution through resolve_lockfile_path_for_read. However, it does NOT verify that consumers call LockFile.read() -- the canonical load authority that now gates all lockfile loads through has_conflict_markers before from_yaml parsing -- rather than calling LockFile.from_yaml() directly. The PR correctly fixes lock_export (lock.py:306-307) to route through both resolve_lockfile_path_for_read and LockFile.read, closing the export bypass that previously called get_lockfile_path + LockFile.from_yaml. But src/apm_cli/commands/lock.py is absent from _LOCKFILE_CONSUMERS, so the fix has no static regression protection. The mutation case (test_architecture_owner_rule_mutations.py:151) proves the guard has teeth for the read_only migration gate, but no mutation case covers a LockFile.read bypass or the lock_export consumer. Per the single-owner rule, every fix without a dual guardrail (behavioral + static) will silently regress. Follow-up should: (1) add lock.py to _LOCKFILE_CONSUMERS, (2) extend the guard to verify LockFile.read usage where from_yaml bypass is structurally reachable, and (3) add a mutation case that kills the LockFile.read delegation in one consumer. Design patterns -- Used in this PR: Base class + subclass (LockfileConflictError extends LockfileFormatError, enabling callers to catch at the granularity they need). Template Method (implicit): LockFile.read is the single entry that gates all loads through conflict detection before from_yaml, enforcing the canonical-owner rule at runtime. Pragmatic suggestion: none -- the current hierarchy is the simplest correct design at this scope.
  Suggested: Add 'src/apm_cli/commands/lock.py' to _LOCKFILE_CONSUMERS; extend check_lockfile_read_resolution to verify LockFile.read usage (not just resolve_lockfile_path_for_read); add a MutationCase that kills the LockFile.read delegation.
  Evidence: unknown / static.

• [nit] _command_path couples CWD-relative rendering into LockfileConflictError domain exception at src/apm_cli/deps/lockfile.py:81
  _command_path (lockfile.py:81-93) computes os.path.relpath at exception-construction time and bakes the result into the error message string. This embeds a presentation concern (rendering git checkout commands relative to the invoking directory) into a domain exception. Currently tolerable: one raiser (LockFile.read:1083), two catchers (enforce_frozen in service.py:287 wraps the message; lock_export in lock.py:307 lets it propagate). If additional callers need to customize the rendered advice (e.g. a future IDE integration or JSON-output mode), the fixed string prevents re-rendering. At current scope no extraction is warranted; note for future refactoring if a third distinct rendering context appears.

test-coverage-expert

• [recommended] Preservation assertions use read_text (universal-newline mode), cannot detect CRLF re-encoding. at tests/integration/test_install_conflicted_lockfile_e2e.py:108
  Both the integration e2e test (line 108) and the spec conformance test (line 852) assert lockfile preservation via Path.read_text(encoding='utf-8'), which applies Python universal-newline translation (\r\n -> \n on all OSes, not just Windows). The PR body and req-lk-023(c) promise 'byte-for-byte' preservation, but this assertion would pass even if a regression rewrote the file with different line endings. A read_bytes() comparison would be strictly stronger. Probed: grep'd all three new test files for read_bytes and newline= -- zero matches for lockfile preservation; the conformance file uses read_bytes only for unrelated trust-archive paths (lines 241, 288, 652, 672).
  Suggested: Replace the read_text preservation assertion with read_bytes() and compare against _CONFLICTED_LOCKFILE.encode('utf-8') in at least one parametrized case, e.g.: assert (conflicted_project / 'apm.lock.yaml').read_bytes() == _CONFLICTED_LOCKFILE.encode('utf-8').
  Evidence: unknown / integration-with-fixtures.

• [recommended] Conformance test for req-lk-023(c) does not assert command outcome (exit code or exception type). at tests/spec_conformance/test_lockfile_reqs.py:849
  test_lockfile_is_left_unmodified_across_reading_operations invokes four command sets with catch_exceptions=True (line 849) but never asserts result.exit_code or isinstance(result.exception, LockfileConflictError). The preservation assertion alone would pass under a regression that changes the diagnostic shape (e.g. raw YAMLError instead of named LockfileConflictError) or even if the command silently succeeds and happens not to write the file. The companion e2e test does assert exit_code == 1, but the conformance test -- the one mapped to the ratified spec clause -- does not carry a parallel outcome gate. Probed: grep'd test_lockfile_reqs.py for exit_code and exception near the req-lk-023(c) test -- zero matches inside that test function.
  Suggested: After the invoke, add: assert result.exit_code != 0, f'{args} MUST fail when the lockfile contains conflict markers'.
  Evidence: unknown / integration-with-fixtures.

• [recommended] No integration test exercises a conflicted lockfile in global or ancestor scope. at tests/integration/test_install_conflicted_lockfile_e2e.py
  All new tests create a project-scope apm.lock.yaml in tmp_path. The production code _command_path() renders an absolute path when the lockfile is outside the working directory (tested at unit tier in test_recovery_command_targets_a_lockfile_outside_the_working_directory), but no Click CliRunner test exercises apm install -g or an ancestor lockfile via the full command flow. Probe: grep'd tests/ for global.*conflict, conflict.*global, --global.*conflict, -g.*conflict -- only unrelated MCP conflict detection and Docker installer files matched (tests/unit/test_conflict_detection.py, tests/unit/test_docker_args_and_installer.py).
  Suggested: Add a parametrized case to the e2e test that places the conflicted lockfile at a user-scope path and invokes install -g, asserting exit_code == 1 and the absolute path in the diagnostic.
  Evidence: missing / integration-with-fixtures.

devx-ux-expert

• [recommended] Reference docs describe the diagnostic as naming a specific git command, coupling prose to the current recipe. at docs/src/content/docs/reference/lockfile-spec.md:412
  req-lk-023 clause (b) deliberately says 'resolve the conflict in that file or restore a known-good lockfile' -- generic, not git-specific. lockfile-spec.md line 412 and install.md line 165 both say 'name the git command that keeps one side of the merge', tying the user-facing reference doc to the current git-checkout recipe. If the recipe is revised to generic resolve/restore guidance, these doc paragraphs require parallel edits. Reference docs are what a confused user reads first; they should reflect the spec's broader framing rather than pin a single recovery path.
  Suggested: Replace 'name the git command that keeps one side of the merge so you can reinstall from it' with language closer to the spec: 'direct you to resolve the conflict or restore a known-good lockfile before reinstalling'. Apply the same change in install.md's frozen-mode bullet.
  Evidence: unknown / static.

• [recommended] Troubleshooting page drops the mid-merge caveat from the recovery recipe. at docs/src/content/docs/troubleshooting/install-failures.md:185
  The CLI diagnostic at lockfile.py:104 says '# or --theirs, mid-merge only' but install-failures.md line 185 shows only '# or --theirs'. git checkout --ours/--theirs requires an active merge; outside an active merge git returns 'error: --ours/--theirs is incompatible with switching branches'. A user who committed the conflicted file and encounters this page later gets a confusing git error with no fallback guidance. The CLI is more honest about the precondition; the doc should match or offer an alternative for the post-merge case.
  Suggested: Add '# mid-merge only' to the comment, matching the CLI output, or add a sentence noting that outside an active merge the user can restore from version control instead (e.g., git checkout main -- apm.lock.yaml).
  Evidence: unknown / static.

• [nit] Recovery recipe does not quote the lockfile path in the printed git command. at src/apm_cli/deps/lockfile.py:104
  The _command_path helper returns an unquoted path string. For the common project-relative apm.lock.yaml this is safe, but a user-scope lockfile under a home directory with spaces (e.g., C:\Users\John Doe...) produces a git command that splits on whitespace. The printed recipe is the one concrete action the error offers; a broken command on the first try undermines the recovery UX.
  Suggested: Quote the target path in the printed recipe, e.g., git checkout "{target}" --ours, or use shlex.quote(target) for the display string.
  Evidence: unknown / static.

supply-chain-security-expert

• [recommended] Shell recipe interpolates the lockfile path unquoted into a git checkout command. at src/apm_cli/deps/lockfile.py:104
  LockfileConflictError.init embeds _command_path(path) directly into the git checkout recipe without shell quoting. If the resolved path contains spaces or shell metacharacters such as dollar signs, a user who copy-pastes the advice gets a broken or misinterpreted command. The test helper diagnostic_recipe.py already imports shlex and splits safely, but the production side that composes the recipe does not quote. shlex.quote(target) would close this gap for all path spellings.
  Suggested: Add 'import shlex' to module imports; change line 104 to: f" git checkout {shlex.quote(target)} --ours # or --theirs, mid-merge only\n"
  Evidence: manual / static.

doc-writer

• [recommended] Replace the unconditional recovery recipe with context-safe manual guidance. at docs/src/content/docs/troubleshooting/install-failures.md:185
  The troubleshooting recipe and packages/apm-guide/.apm/skills/apm-usage/troubleshooting.md:14 omit the 'mid-merge only' qualification now present in src/apm_cli/deps/lockfile.py:104. Conflict markers can survive in a committed file with no unmerged index stages, so selecting --ours or --theirs is not a general repair. The basename also assumes the lockfile is in the current directory. The subsequent plain 'apm install' loses the original command and scope: src/apm_cli/commands/install.py:1357 selects project scope unless --global is supplied, so a user-scope failure can lead to installing an unrelated project. The frozen PR body further contradicts this head by saying the diagnostic 'deliberately names no command'. These are static observations; no recipe or contribution tests were executed.
  Suggested: Make manual resolution of the named file or restoration of a known-good lockfile the general instruction, followed by retrying the original invocation with its scope and flags. If retaining a Git example, explicitly restrict it to an active merge in the owning repository. Align the canonical diagnostic, troubleshooting guide, agent guide, reference summaries, changelog, and PR body without adding a recovery command or policy.
  Evidence: unknown / static.

• [recommended] Do not promise that selecting one side preserves all pins through reinstall. at docs/src/content/docs/troubleshooting/install-failures.md:189
  The claim that reinstall 'resolves only what the merge added' is stronger than the existing replay contract: docs/src/content/docs/reference/cli/install.md:162 limits locked-commit reuse to unchanged Git dependencies. A merge can change an existing dependency's ref, not just add dependencies. Selecting one whole lockfile side also drops records unique to the other side; the agent-guide troubleshooting row's assurance that pins and deployment records 'are not lost' obscures that distinction. The new recipe test invokes run_recipe with only='git' and then checks that LockFile.read succeeds; it does not run the follow-up install or establish post-repair pin preservation. No tests were executed in this review.
  Suggested: State that APM leaves the conflicted file unchanged until the user repairs it. Explain that selecting a side retains only that side's records and that changed or missing dependencies may resolve again; require reviewing the repaired lockfile against the merged manifest before committing. Replace the unconditional agent-guide assurance with the same bounded statement and link to the existing replay contract.
  Evidence: unknown / static.

• [recommended] Document the newly supported legacy lockfile input for read-only export. at docs/src/content/docs/reference/cli/lock.md:79
  The export reference still says it reads 'apm.lock.yaml only'. At this head, src/apm_cli/commands/lock.py:306 calls resolve_lockfile_path_for_read(project_root, read_only=True), whose implementation in src/apm_cli/deps/lockfile.py:1272-1282 selects apm.lock when apm.lock.yaml is absent without renaming it. The same stale input restriction remains in both export rows of packages/apm-guide/.apm/skills/apm-usage/commands.md:440 and :442 and in the export help at src/apm_cli/commands/lock.py:252. Readers with a legacy lockfile are not told that they can export directly without first generating or migrating a lockfile. This is verified from source only; no export invocation or tests were executed.
  Suggested: State once in the export reference that apm.lock.yaml takes precedence and legacy apm.lock is read in place when the current filename is absent. Align the CLI help and agent-guide export descriptions with that behavior, preserving the no-resolution, no-rehash, and no-network guarantees.
  Evidence: unknown / static.

performance-expert

No findings.

cli-logging-expert

No findings.

spec-swagger-editor -- shocked_meter 8/10, confidence high

Summary: Clean addition. req-lk-023 is correctly indexed, counted, and classified across all six locations; RFC 2119 discipline is sound. One recommended follow-up: the section heading under which it sits does not cover conflict-marker detection.

New recommended findings (1)

• [sw-rec-r1-1] sec.5.4 / Appendix D req-lk-023 row -- req-lk-023 is placed in Section 5.4 whose heading is 'Lockfile versions (1, 2) and bumping rules'. The requirement governs merge-conflict-marker detection, which is a pre-parse concern orthogonal to lockfile schema versioning (req-lk-002 / req-lk-004). The Appendix D section column faithfully records '5.4', but a consumer implementer looking for conflict-marker requirements would not search a version-bumping section. The brief notes the title was deliberately left unchanged to avoid slug breakage, so this is a follow-up concern rather than a fold-now item.
  Recommended fix: In a follow-up revision (not this PR), consider either renaming Section 5.4 to a broader title such as 'Lockfile load-time conditions' or relocating req-lk-023 to its own subsection (e.g. 5.4.1) with a heading that names the concern.

New nit findings (1)

• [sw-nit-r1-1] req-lk-023 Note paragraph (spec line 1179) -- The informative 'Note -- that parenthetical is an interim illustrative aid ...' sits between the umbrella MUST clause and the sub-clause enumeration (a)/(b)/(c). This is the only occurrence of the 'Note --' pattern in the spec. Interposing an informative paragraph between 'MUST satisfy all of the following:' and the first lettered clause interrupts the normative flow a reader expects to follow immediately.
  Recommended fix: Move the Note to immediately after sub-clause (c) and the trailing scope paragraph (before Section 5.5), or convert it to a Markdown blockquote (> Note: ...) so it is visually distinct from normative text.

Preserved strengths confirmed

• Count consistency: Sec 1.3, Appendix C trailer, and Appendix D revision-history all agree on 124 normative statements (119 MUST, 5 SHOULD); the Appendix D table row count independently confirms 119+5=124.
• Conformance class enumeration: req-lk-023 is correctly classified as consumer MUST in Section 5.7, Section 11.3.2 (Appendix C Consumer list), Appendix D table, CONFORMANCE.json, CONFORMANCE.md, and requirements.yml.
• Anchor stability: req-lk-023 takes the next free numeric slot (after req-lk-022) with no renumbering of existing ids.
• RFC 2119 keyword discipline: every normative claim in the new text carries an explicit MUST, MUST NOT, or MAY; no lowercase informal usage of normative keywords in the added text.
• Cross-reference accuracy: all six #req-lk-023 anchor references across Section 5.7, Section 11.3.2, Appendix C listing, and Appendix D table resolve to the single tag in Section 5.4.

spec-oci-editor -- shocked_meter 8/10, confidence high

Summary: req-lk-023 is a clean diagnostic-only amendment. Hash envelopes, content addressing, extraction caps, and the supply-chain threat model are untouched. Clause (c) correctly preserves lockfile bytes.

New nit findings (1)

• [oci-nit-r1-1] req-lk-023 -- The freestanding 'Note -- that parenthetical is an interim illustrative aid' paragraph between req-lk-023's opening MUST and clause (a) does not use the blockquote '> Editorial note.' convention established throughout the spec (Sections 5.6.4, 8.5.1, 8.5.5, and twelve other instances). Consistent blockquote formatting would help implementers distinguish non-normative guidance from normative text when scanning clause boundaries.
  Recommended fix: Reformat the Note paragraph as a blockquote: '> Note. That parenthetical is an interim illustrative aid; the normative minimum marker grammar is not yet pinned by this specification.'

Preserved strengths confirmed

• Hash envelope anchoring (req-lk-016) intact: sha256:[0-9a-f]{64} pattern untouched by this amendment
• Canonical content addressing (req-lk-012, req-lk-015) with tree_sha256 edge-case enumeration intact
• Fail-closed extraction (req-sc-004, req-sc-005) with media-type pinning and decompression caps intact
• Supply-chain threat model (Section 10) with full req-XXX mapping intact; req-lk-023 correctly omitted from Section 10.4 because conflict markers are a VCS workflow hazard not an adversarial vector
• Clause (c) of req-lk-023 correctly prevents silent lockfile regeneration which would undermine recorded-hash integrity

spec-pkgmgr-editor -- shocked_meter 8/10, confidence high

Summary: req-lk-023 is a well-scoped diagnostic-only amendment that preserves lockfile determinism and explicitly reserves the recovery slot. The sole interoperability gap is the deferred marker grammar; a follow-up amendment pinning the minimum set would close it. No blocking findings.

New recommended findings (1)

• [pkg-rec-r1-1] req-lk-023 clause (a) -- The marker detection grammar is explicitly deferred ('the normative minimum marker grammar is not yet pinned by this specification'), so two conforming consumers can disagree on whether a lockfile contains conflict markers. While real git conflict markers always produce invalid YAML and the practical divergence is limited to error-message quality, the gap means a conformance test suite cannot black-box verify clause (a) against edge-case inputs (e.g., pipe markers, markers inside YAML literal block scalars, or unusual whitespace after the seven-character prefix). A follow-up amendment pinning at least the three canonical git marker prefixes with their line-start anchor would close this interop gap and make the conformance oracle deterministic.
  Recommended fix: In a follow-up amendment (not this PR), add a normative minimum marker set to req-lk-023 clause (a), for example: 'At minimum, lines beginning with seven consecutive U+003C, seven consecutive U+003E, or seven consecutive U+007C, each followed by U+0020 or end-of-line, MUST be recognised as conflict markers.' Leave the current illustrative parenthetical as-is for now and convert it to normative text in that amendment.

New nit findings (1)

• [pkg-nit-r1-1] req-lk-023, Note paragraph -- The free-standing 'Note -- that parenthetical is an interim illustrative aid' paragraph sits between the MUST lead-in and clause (a) without the spec's established editorial-note markup ('> Editorial note.' or '> Note.'). A reader skimming for normative clauses could misread it as modifying the MUST above.
  Recommended fix: Reformat as a blockquote editorial note consistent with the rest of the document: '> Note. The parenthetical above is an interim illustrative aid; the normative minimum marker grammar is not yet pinned by this specification.'

Preserved strengths confirmed

• Clause (c) preservation contract (MUST NOT remove, rewrite, or re-resolve) prevents silent lockfile mutation, matching the lockfile-determinism discipline of the existing req-lk-005/req-lk-006 surface.
• The requirement is correctly scoped to the consumer conformance class and placed in Section 5.4, consistent with the existing lockfile-load authority chain (req-lk-004).
• The closing paragraph explicitly disclaims recovery authority, functioning as a defensive reserved-slot statement that prevents an implementer from treating this requirement as license for automatic recovery.
• Clause (b) MUST NOT for read-same-file operations is a sound lockfile-UX contract: it prevents the circular-advice anti-pattern where a tool directs the user to an operation that fails on the same unreadable file.

spec-tag-architect -- shocked_meter 8/10, confidence high

Summary: req-lk-023 is well-scoped and correctly integrated across all projection surfaces. Two recommended findings: the section-5.4 heading mismatch dilutes the heading's semantic contract for section-local readers, and the unpinned marker grammar leaves an interoperability gap that should reserve an explicit future-amendment slot. Neither is blocking; the spec remains self-contained and the amendment process is followed cleanly.

New recommended findings (2)

• [tag-rec-r1-1] sec.5.4 / req-lk-023 -- req-lk-023 (conflict-marker detection, a diagnostic requirement) is placed under Section 5.4 whose heading is 'Lockfile versions (1, 2) and bumping rules'. The other two requirements in that section (req-lk-002, req-lk-004) are exclusively about version selection and version refusal. A third-party implementer reading the section heading to locate version-handling obligations would not expect to find a pre-parse diagnostic requirement there. Appendix C and Section 5.7 both index the requirement correctly, so discoverability via the table is intact, but the heading's semantic contract is diluted for any section-local reader. The slug-stability rationale for not renaming the section is understood; the concern is about placement of the requirement, not the title.
  Recommended fix: In a follow-up amendment, insert a subsection heading (e.g. '5.4.1 Pre-parse lockfile conditions') immediately before req-lk-023 to signal the thematic shift within the existing section slug. Alternatively, add a one-sentence editorial bridge after req-lk-004: 'The following requirement addresses a second condition -- unresolved merge-conflict markers -- that prevents the lockfile from being loaded.' Either approach preserves the 5.4 slug while restoring heading-level coherence.
• [tag-rec-r1-2] sec.5.4 / req-lk-023 Note paragraph -- req-lk-023 mandates detection of 'unresolved version-control merge conflict markers' (a MUST), yet the only characterisation of what bytes constitute such a marker is the illustrative parenthetical, which the immediately following Note paragraph disclaims as non-normative. Two conforming implementations can therefore disagree on what byte sequence constitutes a merge-conflict marker and both claim conformance. Git alone produces markers in two-way, diff3, and zdiff3 modes with varying anatomy; other version-control systems use different conventions. The gap is acknowledged and intentional, but the spec gives the implementer no signal about when or how the grammar will be pinned, nor any convergence anchor in the interim.
  Recommended fix: Append a sentence to the Note paragraph reserving the grammar pin as a named future amendment slot, e.g.: 'A normative minimum marker grammar is deferred to a future v0.1.x amendment and will be tracked under the originating issue.' This gives implementers a concrete forward-compatibility signal without pinning any interim detection grammar as a conformance floor.

New nit findings (1)

• [tag-nit-r1-1] sec.5.4 / req-lk-023 Note -- The 'Note -- that parenthetical...' paragraph uses informal inline prose for a normative disclaimer. Other load-bearing notes in the spec use block-quote or explicit callout formatting. Using the same convention here would prevent an implementer from accidentally treating the illustrative parenthetical as the normative detection grammar.
  Recommended fix: Reformat as a block-quote note: '> Note: The parenthetical above is an interim illustrative aid; the normative minimum marker grammar is not yet pinned by this specification.' to match the spec's existing callout conventions.

Preserved strengths confirmed

• Amendment process (Section 9.3, Appendix D): the 0.1.42 entry is dated, cross-referenced, carries a statement-count diff, and follows the established revision-history pattern through 0.1.41.
• Machine-readable projection consistency: requirements.yml, CONFORMANCE.json, CONFORMANCE.md, Appendix C table, and Section 5.7 / 11.3.2 enumerations all agree on req-lk-023 id, keyword (MUST), section (5.4), and conformance class (consumer).
• Diagnosis-only scoping clause at the end of req-lk-023 explicitly disclaims automatic recovery and narrows the requirement to detection and reporting only, protecting forward compatibility for a future recovery amendment.
• Three-clause (a/b/c) structure is self-contained, independently testable, and does not require reading any companion document to implement.